Skip to content

update: add optimism and optimism-sepolia addresses for BTREE token #1143

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged

Conversation

DonLuv
Copy link
Contributor

@DonLuv DonLuv commented May 13, 2025

Description
Add optimism and optimism-sepolia addresses for BTREE token

Tests

Additional context

Metadata

@DonLuv DonLuv requested review from wbnns and a team as code owners May 13, 2025 20:25
Copy link

wiz-inc-a178a98b5d bot commented May 13, 2025

Wiz Scan Summary

Scanner Findings
Vulnerability Finding Vulnerabilities
Data Finding Sensitive Data 1 Low
Secret Finding Secrets
IaC Misconfiguration IaC Misconfigurations
Total 1 Low

View scan details in Wiz

To detect these findings earlier in the dev lifecycle, try using Wiz Code VS Code Extension.

@DonLuv
Copy link
Contributor Author

DonLuv commented May 13, 2025

Not sure why the validate-workflow failed. Let me know if something is malformed in the json or some such, but looks good to me? Leaving the PR here pending feedback in case the CI pipeline is flaky and just needs a kick/re-run or something

@DonLuv
Copy link
Contributor Author

DonLuv commented May 13, 2025

Yeah, I'm not sure what to make of this output from the CI test failing. Not very revealing, but perhaps I'm just looking in the wrong place? @wbnns any advice? Sorry, I know the readme says checks need to pass before a review will be done, but a little unsure what the issue is here:
Screenshot 2025-05-14 at 00 25 30

Decided to run the validation locally to see if I could see what's wrong, and came up with this, but ignore me if I'm going down the wrong path:

used the 'validate' script in the package.json:

"validate": "tsx ./bin/cli.ts validate",

with the same args as the ci seems to be using --datadir ./data --tokens BTREE/
gets to here in the validate.ts file:

which seems not to match on BTREE/, so const folders is an empty list.

if I use BTREE as the arg instead of BTREE/ it matches so const folders isn't empty, but still fails to validate at the end.

Checked the validate.ts cli.ts .circleci/config.yaml and package.json with git blame to see if anything that looked relevant had changed recently, but couldn't see anything that likely would've caused an issue.

Note: I did also try running the generate:ci script first, as it seemed like validate might depend on it. Also, I get this same error when locally reverting my data.json file to not include the changes in this PR, which would make the branch the same as what's already in master, so I may have just overlooked something in my attempt to solve

@tremarkley
Copy link
Collaborator

@DonLuv do you mind rebasing your branch onto the latest version of the repo? there was a bug in the validate script that was fixed with this PR and it looks like your forked repo does not contain the fix: #1144

@wbnns wbnns added the op PR require review from op team label May 28, 2025
@DonLuv
Copy link
Contributor Author

DonLuv commented May 31, 2025

Thanks for the reply! Sure, I'll do that. Will @ you once done

@DonLuv
Copy link
Contributor Author

DonLuv commented May 31, 2025

@tremarkley thanks for the feedback! merged instead of rebased, but seems fixed 😄

Apologies if I came off grouchy in my discussion created on the feedback board. I know y'all must be busy, and appreciate the work open source contributors like yourself and @wbnns do. Was just trying to get a resolution. More PRs for same token to more optimism chains incoming 😄

@DonLuv
Copy link
Contributor Author

DonLuv commented Jun 12, 2025

@tremarkley @wbnns are there any more issues that need to be resolved with this PR? Looks like the ci passed with the update 2 weeks ago. Can we get it merged please?

Copy link
Collaborator

@fainashalts fainashalts left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@fainashalts
Copy link
Collaborator

@DonLuv this looks good to me, sorry for the delay!

@fainashalts fainashalts merged commit 992065b into ethereum-optimism:master Jun 13, 2025
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
op PR require review from op team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants